Join collection matrix variable values with a comma by kdelay · Pull Request #1421 · spring-cloud/spring-cloud-openfeign

kdelay · 2026-09-18T02:44:55Z

MatrixVariableParameterProcessor builds the path segment from each value's toString(), so a collection comes out bracketed: @MatrixVariable("colours") List<String> of red, blue gives ;colours=[red, blue]. A matrix variable separates repeated values with a comma, so WebUtils.parseMatrixVariables reads that as {colours=[[red, blue]]} and ;colours=red,blue as {colours=[red, blue]}. Values are now joined with a comma; arrays and nested values take the same path.

Feign also pct-encodes values substituted into a URI template. The ;name= prefix now sits in the template and stays literal: through a stub Client a String param was %3Bparam%3Dvalue, now ;param=value.

Map variables are unchanged; a collection-valued entry needs a fix in Feign's path-style expansion.

verify: 448 tests, 0 failures.

ryanjbaxter · 2026-09-18T13:11:21Z

Since this changes the return format, I do not want to put this in 5.0.x but it is OK to put in main

kdelay · 2026-09-18T13:20:53Z

Retargeted to main and rebased onto d4d8148 (0e58e3a). Re-ran on that base: reverting only MatrixVariableParameterProcessor leaves the two added tests failing (SpringMvcContractTests:832 and :840) with the rest of the 71 passing; with the change, ./mvnw -P spring -pl spring-cloud-openfeign-core verify gives 445 tests, 0 failures, 0 Checkstyle violations.

ryanjbaxter · 2026-09-18T13:27:24Z

Please sign your commit so the DCO passes

MatrixVariableParameterProcessor built the path segment from the raw toString() of each value, so a collection came out bracketed: ;colours=[red, blue] instead of ;colours=red,blue. A matrix variable separates repeated values with a comma, so the receiving side reads "[red" and " blue]" back out of the bracketed form. Both branches of the processor were affected, including the Map<String, List<String>> signature used as the @MatrixVariable example in the reference documentation. Signed-off-by: kdelay <kdelay20@gmail.com>

kdelay · 2026-09-18T13:54:30Z

Re-pushed the commit (bd2b41f, same tree as 0e58e3a) with the sign-off. DCO is green now.

ryanjbaxter · 2026-09-18T14:10:44Z

 		assertThat(";param=value").isEqualTo(data.indexToExpander().get(0).expand(testMap));
 	}

+	@Test


Can we add a test through a real Feign client and/or RestTemplate? I believe RequestTemplateFactoryResolver.expandElements() will add a , in the result as well

ryanjbaxter · 2026-09-18T14:11:56Z


+	private String expandValue(Object value) {
+		if (value instanceof Collection<?> values) {
+			return values.stream().filter(Objects::nonNull).map(Object::toString).collect(Collectors.joining(","));


I think you can use org.springframework.util.StringUtils.collectionToCommaDelimitedString

Use StringUtils.collectionToCommaDelimitedString for the join and flatten arrays and nested collections through the same path. Signed-off-by: kdelay <kdelay20@gmail.com>

kdelay · 2026-09-18T14:23:39Z

Done in 4bb1784: join via StringUtils.collectionToCommaDelimitedString, arrays through the same path (CollectionUtils.arrayToList), so nested collections/arrays flatten too. Reverting only the processor leaves the 4 added tests failing and the other 69 passing; verify is 447 tests, 0 failures, 0 Checkstyle violations.

You are right about expandElements. With a stub Client a List param gives /matrixVariable/%3Bcolours%3Dred,%3Bcolours%3Dblue, a Map param %3Bcolours%3Dred%2Cblue%3Bsize%3DL, on main too. Should I open a separate issue for that?

ryanjbaxter · 2026-09-18T14:29:47Z

You are right about expandElements. With a stub Client a List param gives /matrixVariable/%3Bcolours%3Dred,%3Bcolours%3Dblue, a Map param %3Bcolours%3Dred%2Cblue%3Bsize%3DL, on main too. Should I open a separate issue for that?

No, IMO we should deal with it as part of this change

Feign pct-encodes every value it substitutes into a URI template, so the ';' and '=' produced by the expander left the segment as %3Bname%3Dvalue and the server could not read it as matrix variables. Move the ';name=' prefix into the URI template, where it survives as a literal, and let the expander produce only the value. A collection parameter is expanded element by element by Feign and joined with ',', which now yields ';colours=red,blue'. Signed-off-by: kdelay <kdelay20@gmail.com>

kdelay · 2026-09-18T14:44:54Z

Done in 02fbad7. Feign pct-encodes every value substituted into a URI template, so the separators came out as %3B/%3D. The ;name= prefix now lives in the template, where it stays literal.

Measured through a stub Client (2 tests added): String was %3Bparam%3Dvalue, now ;param=value; List was %3Bcolours%3Dred,%3Bcolours%3Dblue, now ;colours=red,blue.

Arrays (not Iterable, so joined inside the value) and Map (dynamic keys) still encode; both need reserved characters inside a value, which UriTemplate forbids.

verify: 449 tests, 0 failures.

ryanjbaxter · 2026-09-18T16:05:54Z

 	}

+	private String expandValue(Object value) {
+		if (value.getClass().isArray()) {


I don't think the Map-typed @MatrixVariable case case is handled properly

Let Feign expand a Map typed matrix variable through a path-style URI template expression, so the ; and = separators stay literal in the request URL and only the keys and the values are encoded. Signed-off-by: kdelay <kdelay20@gmail.com>

kdelay · 2026-09-19T00:37:19Z

Done in 94a9ad3: the Map variable is now a path-style expression ({;params}) expanded by Feign, so ; and = stay literal. {"colours":"red","size":"L"} was %3Bcolours%3Dred%3Bsize%3DL, now ;colours=red;size=L. verify: 449 tests, 0 failures.

Collection-valued entries are still not covered: expandMap encodes each value without recursing, so ["red","blue"] gives ;colours=%5Bred%2C%20blue%5D, and joining them myself yields red%2Cblue, a single value. The reserved-safe form is a repeated name, which Feign emits for a top-level Iterable only. I can raise that with Feign.

ryanjbaxter · 2026-09-21T19:05:06Z

Shouldn't colours=[red,blue], size=L result in ;colours=red,blue;size=L? This code still seems to return ;colours=[red, blue];size=L. Could you add a test for this?

kdelay · 2026-09-22T00:35:24Z

Measured on feign-core 13.14 through a stub Client: a literal , between repeated values only comes from Feign joining an Iterable argument itself. A Map reaches the expander as one object and Feign pct-encodes the result, so ;colours=red,blue;size=L comes out as %3Bcolours%3Dred%2Cblue%3Bsize%3DL. Path-style keeps ; and = literal but encodes each value: "red,blue" becomes red%2Cblue. indexToEncoded is never read in 13.14.

That output needs a Feign-side change to path-style expansion. Open it upstream and keep this PR to the cases it covers, or drop the map change?

ryanjbaxter · 2026-09-22T13:01:42Z

Thanks for the explaination.

Lets drop the Map coverage for now and file the bug upstream in Feign.

Feign encodes each value of a path-style expression, so a collection valued map entry cannot keep its separators. Leave map expansion as it was and keep this change to the cases it covers. Signed-off-by: kdelay <kdelay20@gmail.com>

kdelay · 2026-09-22T13:20:18Z

Done in f404d40: map expansion and its tests are back to what they are on main. The change now covers single values, collections and arrays.

./mvnw -P spring -pl spring-cloud-openfeign-core verify: 448 tests, 0 failures, 0 Checkstyle violations. Reverting only the processor leaves the 6 matrix variable tests failing and the other 68 passing.

I will open the path-style expansion bug with Feign and link it here.

spring-cloud-issues added the waiting-for-triage label Sep 18, 2026

kdelay force-pushed the fix/matrix-variable-collection-values-50x branch from f1b2108 to 0e58e3a Compare September 18, 2026 13:20

kdelay changed the base branch from 5.0.x to main September 18, 2026 13:20

kdelay force-pushed the fix/matrix-variable-collection-values-50x branch from 0e58e3a to bd2b41f Compare September 18, 2026 13:54

ryanjbaxter requested changes Sep 18, 2026

View reviewed changes

Expand array and nested matrix variable values

4bb1784

Use StringUtils.collectionToCommaDelimitedString for the join and flatten arrays and nested collections through the same path. Signed-off-by: kdelay <kdelay20@gmail.com>

ryanjbaxter requested changes Sep 18, 2026

View reviewed changes

Comment thread ...in/java/org/springframework/cloud/openfeign/annotation/MatrixVariableParameterProcessor.java

ryanjbaxter requested changes Sep 18, 2026

View reviewed changes

Drop map matrix variable coverage

f404d40

Feign encodes each value of a path-style expression, so a collection valued map entry cannot keep its separators. Leave map expansion as it was and keep this change to the cases it covers. Signed-off-by: kdelay <kdelay20@gmail.com>

Conversation

kdelay commented Sep 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ryanjbaxter commented Sep 18, 2026

Uh oh!

kdelay commented Sep 18, 2026

Uh oh!

ryanjbaxter commented Sep 18, 2026

Uh oh!

kdelay commented Sep 18, 2026

Uh oh!

ryanjbaxter Sep 18, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ryanjbaxter Sep 18, 2026

Choose a reason for hiding this comment

Uh oh!

kdelay commented Sep 18, 2026

Uh oh!

Uh oh!

ryanjbaxter commented Sep 18, 2026

Uh oh!

kdelay commented Sep 18, 2026

Uh oh!

ryanjbaxter Sep 18, 2026

Choose a reason for hiding this comment

Uh oh!

kdelay commented Sep 19, 2026

Uh oh!

ryanjbaxter commented Sep 21, 2026

Uh oh!

kdelay commented Sep 22, 2026

Uh oh!

ryanjbaxter commented Sep 22, 2026

Uh oh!

kdelay commented Sep 22, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

kdelay commented Sep 18, 2026 •

edited

Loading